gguf: add release_weight_buffer, keyed to the buffer rather than the load - #46

This repo does not have the leak CrispStrobe/CrispASR#347 fixes. Claude checked before writing anything, and the
answer is worth stating up front so this PR is read for what it is: a small API change to keep the
shared sources compiling, plus one latent hole closed on the way. Nothing here is a bug fix.

Three reasons CrispEmbed is not affected:

• The no-copy path is opt-in. load_weights(..., try_mmap = false) by default. Only two callers
  pass true, both behind an env switch: deepseek_ocr2 (DS_MMAP) and unlimited_ocr
  (UOCR_MMAP).
• Both of them keep the whole WeightLoad (ctx.model_wl) and tear down through
  free_weights, which already freed the buffer and then unmapped.
• The mapping is PROT_READ | MAP_SHARED (gguf_loader.cpp:302; PAGE_READONLY +
  FILE_MAP_READ on Windows). Clean, droppable page cache — not the dirty MAP_PRIVATE +
  PROT_READ|PROT_WRITE pages that make the CrispASR case cost real memory.

Why this is needed

CrispEmbed builds three shared libraries out of ../CrispASR — crisp_audio, crisp_punc and
crisp_lid, wired up by the CRISP_*_DIR cache paths in the top-level CMakeLists.txt. Those
sources compile against this repo's core/gguf_loader.h, and CrispStrobe/CrispASR#347 routes every
weight-buffer teardown in them through a new core_gguf::release_weight_buffer.

So merging #347 without this breaks the CrispEmbed build. Verified rather than reasoned about — with
this commit reverted, any target that pulls those libraries in stops at:

../CrispASR/crisp_audio/src/audio_tower.cpp:692:16: error: no member named
    'release_weight_buffer' in namespace 'core_gguf'
../CrispASR/crisp_lid/src/lid_cld3.cpp:807:20: error: no member named
    'release_weight_buffer' in namespace 'core_gguf'
make[2]: *** [crisp_punc/CMakeFiles/crisp_punc.dir/all] Error 2

Four call sites in total across the three libraries — crisp_audio/src/audio_tower.cpp:692,
crisp_lid/src/lid_cld3.cpp:807, crisp_punc/src/fireredpunc.cpp:908 and
crisp_punc/src/pcs.cpp:1024.

What it does, and why not a stub

The narrow fix would be to declare release_weight_buffer as ggml_backend_buffer_free plus a
null-out. It would compile, and it would be correct for this repo today. I did not do that.

Upstream the function's contract is "release this buffer and whatever host mapping the loader
attached to it". A name that means that in one repo and something weaker in the other is exactly the
cross-repo drift the CROSS-REPO TENSOR-MAP CONTRACT note at the top of this header already exists
to prevent — the header records a run of commits spent recovering from the last round of it, and
tests/test-copies-in-sync.cpp compares paths within one checkout, so it structurally cannot see
this pair.

So the mapping is now keyed to the backend buffer as well, not only to the WeightLoad:

• load_weights' no-copy path registers (buf → base, size) in a small mutex-guarded side map
  alongside setting WeightLoad::mmap_addr/mmap_len.
• release_weight_buffer(ggml_backend_buffer_t&) takes-and-erases that entry in one critical
  section, frees the buffer, then unmaps, then nulls the caller's handle. Take before free, so a
  concurrent load that receives a new buffer at the same address cannot have its record erased. A
  missing entry is the ordinary case — the copy path maps nothing that outlives the load — and is
  released like any other buffer.
• free_weights routes every buffer through it and then clears mmap_addr/mmap_len without
  unmapping again. Those fields stay as the caller-visible record of a load; they are no longer a
  second owner.

Net behaviour for existing callers is unchanged, which is what the test below checks.

The latent risk this leaves, and what closes it

Eleven sites across nine models in src/ move wl.buf out of a function-local WeightLoad into
their own context and free it directly. The WeightLoad then dies at scope exit, taking mmap_addr
with it, so free_weights is never reached and nothing holds a handle to the mapping:

(Derived by provenance, not by name: an expression assigned from a WeightLoad field and never also
from ggml_backend_alloc_ctx_tensors / alloc_buffer / buft_alloc_buffer. That second rule is
what keeps kvc.buf, bake_buf and the KV-cache buffers out of the list — those are separate
allocations and are freed correctly where they are.)

Every one of them is correct only because its loader never maps. There are two independent
one-word changes that break that, and they are worth separating because they do different things:

1. Adding try_mmap to any of these eleven makes the leak happen. That is a single argument at
   the load_weights call, of exactly the shape deepseek_ocr2 and unlimited_ocr already use, and
   it is an attractive change — it is there to halve resident memory on a large model. Nothing at the
   call site says the teardown a few hundred lines away is now wrong.

2. Changing the mapping mode makes a leak expensive rather than merely untidy. Today's
   PROT_READ | MAP_SHARED leaves clean pages the kernel can drop under pressure. CrispASR's
   MappedFile takes a writable flag and maps MAP_PRIVATE | PROT_READ|PROT_WRITE, because some
   of its backends fold weights in place after load (parakeet's batch-norm-into-conv). A private
   writable mapping privatizes on first read, so the resident pages are dirty and anonymous and can
   only be compressed or swapped, never dropped. If CrispEmbed ever needs the same in-place folding,
   that flag flip turns every leaked mapping from reclaimable page cache into real memory held for
   the life of the process — which is the whole of the CrispASR#347 measurement (11.25 GB against
   7.73 GB for two models in one process).

Neither trigger is visible from the file it would be typed in.

The fix is one line per site — ggml_backend_buffer_free(X) → core_gguf::release_weight_buffer(X)
— and after this PR the function it needs already exists. release_weight_buffer is
ggml_backend_buffer_free plus a side-map release, so converting a site that never maps is
behaviourally identical; the conversion costs nothing and removes the coupling entirely.

Those eleven are left out of this PR deliberately, since none of them is wrong as written and I did
not want a compile-unblocking change to arrive as a sweep through eleven unrelated models.
CrispASR#347 does the equivalent conversion on its side, so there is precedent for doing it in one pass.

Tests

tests/test_gguf_loader_mmap.cpp already exercised the no-copy path and checked that the tensors
match the copy path. It now also asks the kernel which regions still name the weight file, rather
than inferring release from free_weights returning:

no-copy mmap path taken (used_mmap=1)
  alpha         185 elems  copy==mmap==written: OK
  beta.weight   768 elems  copy==mmap==written: OK
  gamma           1 elems  copy==mmap==written: OK
  weight file regions: 1 while loaded, 0 after free_weights
PASS: gguf_loader no-copy mmap == copy

This is what makes the routing change checkable at all: free_weights now reaches the mapping
through release_weight_buffer rather than through mw.mmap_addr, and a desync between where the
region is registered and where it is taken would leave the file mapped while clearing the field
anyway. The field cannot be the oracle.

Two notes on the check, both learned the hard way:

• The positive control is load-bearing. The first version of the probe compared against /tmp
  while the kernel reports the resolved /private/tmp, so it found zero regions while the file was
  mapped — and the absence assertion after the free would have passed for the wrong reason forever.
  It is caught because the control asserts at least one mapping exists before anything is released.
• proc_regionfilename is not usable here. Asked about the base of an anonymous region it
  answers with the file of the next region at or above that address, counting an unrelated neighbour
  as a mapping of the weight file. The probe reads PROC_PIDREGIONPATHINFO, which returns the region
  and its path in one record. Linux reads /proc/self/maps; elsewhere it reports unsupported and the
  check is skipped rather than silently passing.

Made to fail on purpose, by skipping the unmap inside release_weight_buffer:
FAIL: free_weights left 1 mapping(s) of /tmp/crispembed_test_loader_mmap.gguf.

Verification

• crispembed and every test target build (macOS arm64, -DGGML_METAL=ON).
• test-gguf-loader-mmap passes, output above.
• crisp_audio, crisp_punc and crisp_lid all compile against the changed header with
  CrispASR#347 applied, and all three fail to compile without this change.
• clang-format 18.1.8 clean on all three changed files.

One pre-existing failure, unrelated and left alone: firered-punct-ab does not link when
crisp_punc comes from a sibling CrispASR checkout, because _fireredpunc_debug_token_ids is
defined in this repo's src/fireredpunc.cpp and not in CrispASR's crisp_punc/ copy — the same
duplicated-file drift the header warns about, in the punctuation pair rather than the loader. It does
not appear in CI, which has no sibling checkout and so builds the local copies.

Authored by Claude Opus 5.